Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improvement: Moving secrets from USER_SETTINGS.cpp to platformio config #662

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

No-Signal
Copy link

What

This PR moves user secrets from USER_SETTINGS.cpp & USER_SETTINGS.h into a platformio config file USER_SECRETS.ini

Why

This change makes it less likely that secrets are accidentally exposed by users committing them and makes it easier to maintain these secrets during development

How

  • USER_SECRETS.ini is included as additional platformio configuration in platformio.ini
  • A template file USER_SECRETS_TEMPLATE.ini is copied to USER_SECRETS.ini if it does not exist and acts as a base configuration file
  • USER_SECRETS.ini has been added to .gitignore to ensure it is not accidentally tracked

@obbardc
Copy link
Contributor

obbardc commented Dec 8, 2024

I'm in favour of this !

@dalathegreat
Copy link
Owner

Nice improvement! Does it work good for Arduino IDE users also?

@beadon
Copy link
Contributor

beadon commented Dec 10, 2024

I also like this approach. Testing ..

@No-Signal
Copy link
Author

A different approach would be required to support Arduino IDE users which would require a one time manual action which I avoided with Platformio.

The best option I can see to support both Arduino IDE and Platformio is to create a USER_SECRETS_TEMPLATE.h file that users would need to copy to USER_SECRETS.h once manually. USER_SECRETS.h would be added to .gitignore to avoid accidental commits.

@dalathegreat
Copy link
Owner

Yeah well 99% of users are Arduino IDE users, so I guess we would need to cater to those 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants